Skip to content

Add AbstractThingHandlerDiscoveryService constructor for tests to use#5100

Merged
holgerfriedrich merged 1 commit intoopenhab:mainfrom
Nadahar:enable-mock-abstractthinghandlerdiscoveryservice-executor
Oct 26, 2025
Merged

Add AbstractThingHandlerDiscoveryService constructor for tests to use#5100
holgerfriedrich merged 1 commit intoopenhab:mainfrom
Nadahar:enable-mock-abstractthinghandlerdiscoveryservice-executor

Conversation

@Nadahar
Copy link
Contributor

@Nadahar Nadahar commented Oct 25, 2025

@holgerfriedrich Immediately when starting to try to fix the tests, I met one of the situations where I needed the ability to override thingDiscovered(). This is the alternative I've been able to come up with for this particular problem, we need a corresponding "test constructor" in AbstractThingHandlerDiscoveryService as we have in AbstractDiscoveryService.

As usual, I can't keep my fingers off other things I bump into, so I've added some JavaDoc to the constructors, and changed the return type of getThingHandler(). If that's controversial, I can just revert it, but the reason I did it was that when trying to research how to document the parameter in the JavaDoc, I realized that it wasn't actually used for anything (except to enforce the type in setThingHandler() - which serves no purpose alone).

As far as I can understand, this change will only trigger warnings for "unnecessary casts", so it shouldn't break anything. Not having to cast the result should at least make having the parameter there do something useful.

… a different executor, some JavaDocs, and made ThingHandler type generic

Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>
@Nadahar Nadahar requested a review from a team as a code owner October 25, 2025 21:10
@Nadahar
Copy link
Contributor Author

Nadahar commented Oct 25, 2025

I've managed to restore all the tests using the PR: https://github.com/Nadahar/openhab-addons/tree/reenable-discovery-tests

I haven't created a PR yet, because it will only fail to build without this one being merged, but it means that no further changes should be required in Core to make the tests work.

@Nadahar
Copy link
Contributor Author

Nadahar commented Oct 25, 2025

For reference, this is a "continuation of"/related to #5072.

Copy link
Member

@holgerfriedrich holgerfriedrich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@holgerfriedrich holgerfriedrich merged commit 08b3fb2 into openhab:main Oct 26, 2025
4 checks passed
@holgerfriedrich holgerfriedrich added this to the 5.1 milestone Oct 26, 2025
@holgerfriedrich holgerfriedrich added test enhancement An enhancement or new feature of the Core labels Oct 26, 2025
@Nadahar Nadahar deleted the enable-mock-abstractthinghandlerdiscoveryservice-executor branch October 26, 2025 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement An enhancement or new feature of the Core test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants